Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Kolide OSQUERY tables (Part 1+2+3 of 4) #14613

Merged
merged 118 commits into from
Nov 2, 2023
Merged

Conversation

sharon-fdm
Copy link
Collaborator

@sharon-fdm sharon-fdm commented Oct 18, 2023

Adding Kolide tables to orbit

@sharon-fdm sharon-fdm changed the title Add tables Kolide OSQUERY tables in Fleetd (Part 2 of 4, 9 tables) Oct 20, 2023
@sharon-fdm sharon-fdm changed the title Kolide OSQUERY tables in Fleetd (Part 2 of 4, 9 tables) Add Kolide OSQUERY tables (Part 2 of 4, 9 tables) Oct 20, 2023
@mostlikelee mostlikelee temporarily deployed to Docker Hub October 31, 2023 21:41 — with GitHub Actions Inactive
@mostlikelee mostlikelee temporarily deployed to Docker Hub November 1, 2023 13:12 — with GitHub Actions Inactive
Copy link
Contributor

@mostlikelee mostlikelee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@getvictor possibly a remnant from tables we removed late, but I was able to remove the entire execparsers package

@@ -14,6 +14,7 @@ on:

env:
ORBIT_VERSION: 1.17.0
CGO_ENABLED: 1
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@getvictor IIRC it's for building the app_icons table which has a C dependency

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, app_icons have C code that needs to be compiled in. It works on darwin and our CI, but I'm not sure if cross-compiling will work on Windows. @sharon-fdm do we need to support cross-compiling on Windows?

Copy link
Member

@noahtalerman noahtalerman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I proposed some table cuts! They fit a couple themes:

  • Duplicate info. We already have a table for it.
  • Hard to use. The table is difficult to use. We can improve and add it later
  • Value is unclear. We don't understand why a user would want to use this table. Let's learn more before we add these.

changes/14464-add-kolide-tables Outdated Show resolved Hide resolved
changes/14464-add-kolide-tables Outdated Show resolved Hide resolved
changes/14464-add-kolide-tables Outdated Show resolved Hide resolved
changes/14464-add-kolide-tables Outdated Show resolved Hide resolved
changes/14464-add-kolide-tables Outdated Show resolved Hide resolved
changes/14464-add-kolide-tables Outdated Show resolved Hide resolved
changes/14464-add-kolide-tables Outdated Show resolved Hide resolved
changes/14464-add-kolide-tables Outdated Show resolved Hide resolved
changes/14464-add-kolide-tables Outdated Show resolved Hide resolved
changes/14464-add-kolide-tables Outdated Show resolved Hide resolved
@mostlikelee mostlikelee temporarily deployed to Docker Hub November 1, 2023 15:28 — with GitHub Actions Inactive
Copy link
Member

@getvictor getvictor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few files appear to be missing based on comment

orbit/pkg/table/tablehelpers/exec_test.go Show resolved Hide resolved
orbit/pkg/table/tablehelpers/mocks.go Show resolved Hide resolved
@mostlikelee mostlikelee temporarily deployed to Docker Hub November 1, 2023 18:17 — with GitHub Actions Inactive
@mostlikelee mostlikelee temporarily deployed to Docker Hub November 1, 2023 19:39 — with GitHub Actions Inactive
@mostlikelee mostlikelee requested a review from getvictor November 1, 2023 20:07
@mostlikelee mostlikelee temporarily deployed to Docker Hub November 1, 2023 20:14 — with GitHub Actions Inactive
@mostlikelee mostlikelee temporarily deployed to Docker Hub November 1, 2023 21:30 — with GitHub Actions Inactive
Copy link
Member

@getvictor getvictor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mostlikelee mostlikelee dismissed noahtalerman’s stale review November 2, 2023 02:11

tables removed, merging this to get it in QAs hands

@mostlikelee mostlikelee merged commit ab77170 into main Nov 2, 2023
28 checks passed
@mostlikelee mostlikelee deleted the 14464-first-four-tables branch November 2, 2023 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants